adds pagination on Alert Instances list on Alert Details page #57524

gmmorris · 2020-02-13T00:10:14Z

Summary

adds pagination on Alert Instances list on Alert Details page

Checklist

Delete any items that are not applicable to this PR.

~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~[ ] Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-02-13T00:10:17Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

* master: add `absolute` option to `getUrlForApp` (elastic#57193) [Telemetry] Migrate public to NP (elastic#56285) address flaky test where instances might have different start… (elastic#57506) fix(NA): support legacy plugins path in plugins (elastic#57472) build immutable bundles for new platform plugins (elastic#53976) [SIEM] [Detection Engine] Reject if duplicate rule_id in request payload (elastic#57057) Add autocomplete="off" for input type="password" to appease the scanners (elastic#56922) Use default spaces suffix for signals index if spaces disabled (elastic#57244) [Alerting] Create alert design cleanup (elastic#56929)

gmmorris · 2020-02-13T19:31:17Z

@elasticmachine merge upstream

pmuellr

As much as I understand this stuff, LGTM. Made some nit-level comments about the code, and had some questions about testing the result of the pagination which seem like they could be flaky.

...triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.tsx

pmuellr · 2020-02-13T21:08:58Z

x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts

+
+        const { alertInstances } = await alerting.alerts.getAlertState(alert.id);
+
+        const [firstItem] = await pageObjects.alertDetailsUI.getAlertInstancesList();


should it check the # of items in the list? Should be 10, right?

Ah fair, I'll add that 👍

pmuellr · 2020-02-13T21:10:20Z

...triggers_actions_ui/public/application/sections/alert_details/components/alert_instances.tsx

@@ -135,22 +135,36 @@ export function AlertInstances({
  unmuteAlertInstance,
  requestRefresh,
 }: AlertInstancesProps) {
+  const [pagination, setPagination] = useState<Pagination>({ index: 0, size: 10 });


there are a couple of 10 literals throughout the PR; should these be a constant defined somewhere?

Well, only once in the plugin itself, the others are tests where the duplication makes sense to me. But that said, alert list has it too, I'll pull both into a constant.

x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/details.ts

peterschretlen

I tested using 100 alert instances - pagination, changing page size, skipping pages all working as expected. I would agree with Patrick maybe add a const DEFAULT_PAGE_SIZE = 10 otherwise LGTM.

* master: (22 commits) skip flaky suite (elastic#50018) skip settings tests (elastic#57608) skip failing suite (elastic#44631) [SIEM] [Case] Initial UI (elastic#57283) handle viewing sample dashboards on default dist (elastic#57510) Fix detection of "system requests" in plugins (elastic#57149) [ML] New Platform server shim: update job service schema (elastic#57614) skip flaky suite (elastic#44631) [APM] Update monospace font family variable (elastic#57555) skip flaky test (elastic#57377) Skip save query tests (elastic#57589) [Maps] allow simultaneous opening of multiple tooltips (elastic#57226) [Uptime] Fix/host connected components (elastic#56969) [logs][metrics][docs] Update screenshots for 7.6 (elastic#57254) [ML] New Platform server shim: update job service routes to use new platform router (elastic#57403) [Maps] Fix document source top hits split by scripted field (elastic#57481) Use log4j pattern syntax (elastic#57433) [ML] Categorization field example endpoint tests (elastic#57471) [Lens] Filter out pinned filters from saved object of Lens (elastic#57197) Lens client side shim cleanup (elastic#56976) ...

…bana into alerting/instance-pagination * 'alerting/instance-pagination' of github.com:gmmorris/kibana:

YulNaumenko

LGTM

gmmorris · 2020-02-14T00:45:24Z

@elasticmachine merge upstream

mikecote · 2020-02-14T12:30:10Z

@elasticmachine merge upstream

mikecote · 2020-02-14T15:05:32Z

@elasticmachine merge upstream

mikecote · 2020-02-14T18:29:49Z

@elasticmachine merge upstream

…ance-pagination

mikecote · 2020-02-18T18:23:42Z

Running functional tests 42x to ensure no flakiness: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/192/.

kibanamachine · 2020-02-18T20:11:18Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 935d4f2

History

💚 Build #27348 succeeded bfce131
💔 Build #26922 failed 41dc30b
💔 Build #26713 failed e9aa6a1
💔 Build #26632 failed 1ba503c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…c#57524) * added pagination on alert instances page * extracted default page size to a constant for alerting UI as a whole * Fix test failure Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

#58045) * added pagination on alert instances page * extracted default page size to a constant for alerting UI as a whole * Fix test failure Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Mike Côté <mikecote@users.noreply.github.com> Co-authored-by: Gidi Meir Morris <github@gidi.io> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

added pagination on alert instances page

6ca3826

gmmorris added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Feb 13, 2020

gmmorris requested a review from a team as a code owner February 13, 2020 00:10

Merge branch 'master' into alerting/instance-pagination

1ba503c

pmuellr approved these changes Feb 13, 2020

View reviewed changes

YulNaumenko self-requested a review February 13, 2020 21:44

peterschretlen approved these changes Feb 13, 2020

View reviewed changes

gmmorris added 3 commits February 14, 2020 11:38

extracted default page size to a constant for alerting UI as a whole

7930a7c

Merge branch 'alerting/instance-pagination' of github.com:gmmorris/ki…

4bf060f

…bana into alerting/instance-pagination * 'alerting/instance-pagination' of github.com:gmmorris/kibana:

YulNaumenko approved these changes Feb 13, 2020

View reviewed changes

Merge branch 'master' into alerting/instance-pagination

e9aa6a1

Merge branch 'master' into alerting/instance-pagination

64e037b

Merge branch 'master' into alerting/instance-pagination

6dd70ef

elasticmachine and others added 3 commits February 14, 2020 12:29

Merge branch 'master' into alerting/instance-pagination

41dc30b

Merge branch 'master' of github.com:elastic/kibana into alerting/inst…

25e4589

…ance-pagination

Fix test failure

bfce131

Merge with master

935d4f2

mikecote merged commit 85097d5 into elastic:master Feb 19, 2020

mikecote mentioned this pull request Feb 19, 2020

[7.x] adds pagination on Alert Instances list on Alert Details page (#57524) #58045

Merged

mikecote assigned gmmorris Feb 21, 2020

gmmorris added the backported label Mar 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds pagination on Alert Instances list on Alert Details page #57524

adds pagination on Alert Instances list on Alert Details page #57524

gmmorris commented Feb 13, 2020 •

edited

Loading

elasticmachine commented Feb 13, 2020

gmmorris commented Feb 13, 2020

pmuellr left a comment

pmuellr Feb 13, 2020

gmmorris Feb 13, 2020

pmuellr Feb 13, 2020

gmmorris Feb 13, 2020

peterschretlen left a comment

YulNaumenko left a comment

gmmorris commented Feb 14, 2020

mikecote commented Feb 14, 2020

mikecote commented Feb 14, 2020

mikecote commented Feb 14, 2020

mikecote commented Feb 18, 2020

kibanamachine commented Feb 18, 2020


		const { alertInstances } = await alerting.alerts.getAlertState(alert.id);

		const [firstItem] = await pageObjects.alertDetailsUI.getAlertInstancesList();

adds pagination on Alert Instances list on Alert Details page #57524

adds pagination on Alert Instances list on Alert Details page #57524

Conversation

gmmorris commented Feb 13, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 13, 2020

gmmorris commented Feb 13, 2020

pmuellr left a comment

Choose a reason for hiding this comment

pmuellr Feb 13, 2020

Choose a reason for hiding this comment

gmmorris Feb 13, 2020

Choose a reason for hiding this comment

pmuellr Feb 13, 2020

Choose a reason for hiding this comment

gmmorris Feb 13, 2020

Choose a reason for hiding this comment

peterschretlen left a comment

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

gmmorris commented Feb 14, 2020

mikecote commented Feb 14, 2020

mikecote commented Feb 14, 2020

mikecote commented Feb 14, 2020

mikecote commented Feb 18, 2020

kibanamachine commented Feb 18, 2020

💚 Build Succeeded

History

gmmorris commented Feb 13, 2020 •

edited

Loading